Conversation
|
Redoing benchmarks after rebasing now that #24 was merged: Before: After: |
sacckey
left a comment
There was a problem hiding this comment.
I apologize for the delay in the review.
I've left some minor comments, so please make the necessary changes. Thank you.
lib/rubyboy/bus.rb
Outdated
| when 0xd | ||
| return @ram.wram2[addr - 0xd000] | ||
| when 0xf | ||
| case (addr >> 8) & (0xf) |
There was a problem hiding this comment.
Would you mind changing it to be written as follows, without using an ampersand:
when 0xf
case addr >> 8
when 0xfe
~~
when 0xff
~~
|
|
||
| set_methods | ||
| end | ||
| return @apu.read_byte(addr) if last_byte <= 0x26 && last_byte >= 0x10 |
There was a problem hiding this comment.
Just a minor detail, but since it was originally written as when 0xff10..0xff26, it might be more readable to write it starting with the smaller value, like if last_byte >= 0x10 && last_byte <= 0x26.
There was a problem hiding this comment.
It was done this way for performance reasons. Almost all values are >= 0x10 few <= 0x26, so it avoid a comparison in most cases.
If Ruby had a very advanced optimizing compiler that wouldn't be needed, but it's not the case yet.
But if you fell very strongly about it, I can change that.
There was a problem hiding this comment.
I see, got it!
I don't have a strong opinion, so it's fine as is.
Uses the same style as: sacckey#24 main: ``` Ruby: 3.4.1 YJIT: true 1: 3.882817 sec 2: 3.83399 sec 3: 3.835251 sec FPS: 389.5409804902295 Ruby: 3.4.1 YJIT: true 1: 3.981531 sec 2: 3.804446 sec 3: 3.823725 sec FPS: 387.606848134431 ``` This branch: ``` Ruby: 3.4.1 YJIT: true 1: 3.503513 sec 2: 3.437195 sec 3: 3.52252 sec FPS: 430.07760129092094 ``` ``` Ruby: 3.4.1 YJIT: true 1: 3.620516 sec 2: 3.582359 sec 3: 3.484475 sec FPS: 421.05854117250766 ```
sacckey
left a comment
There was a problem hiding this comment.
Thank you for the optimization!
It runs much faster now thanks to you.
I'm planning to release it as version 1.5.1
Uses the same style as: #24
main:
This branch: